Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

remove use of empty var QT_LIBRARIES and use VTK_LIBRARIES and ITK_LIBRARIES… #610

Open
wants to merge 13 commits into
base: master
Choose a base branch
from

Conversation

nbaraka
Copy link

@nbaraka nbaraka commented Apr 16, 2020

To link to specific qt lib : look for qt package with the required lib (for example qtsql).

ITKIOTransformMatlab ITKIOMRC ITKIOTransformInsightLegacy ITKRegistrationCommon
ITKIOMeta ITKStatistics ITKIOPhilipsREC)

#ITKCommon ITKTransformFactory
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to remove

@mathildemerle mathildemerle added this to the 4.0 milestone Apr 16, 2020
nbaraka added 3 commits April 16, 2020 17:03
…nstead of set_definition, removed unecessary link, use target_include_directories instead of include_directories
dtkCoreSupport
dtkLog
medCore
medCoreLegacy
ITKCommon
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i'm curious, why do you prefer them in the find_package than here?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like to put them in the find required section. And I hope that one time I could remove most of them .

target_compile_definitions(${TARGET_NAME} PUBLIC ${TARGET_NAME_UP}_VERSION="${${TARGET_NAME}_VERSION}")

## #############################################################################
## include directorie.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

*directories



## #############################################################################
## add library
## include directorie.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

*directories (there are some changes like that in the PR)

find_package(VTK REQUIRED COMPONENTS vtkIOImage)
include(${VTK_USE_FILE})


Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can remove this empty line

target_compile_definitions(${TARGET_NAME} PUBLIC ${TARGET_NAME_UP}_VERSION="${${TARGET_NAME}_VERSION}")

## #############################################################################
## include directorie.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

directories

@mathildemerle
Copy link
Collaborator

I think there is a problem with the PR @nbaraka you lost all your changes except the last commit :/

nbaraka added 7 commits April 22, 2020 10:28
…de_directories, done for few vtk , still some remains
…de(VTK_file) but have to add VTK_DEFINITIONS; may have to add some ITK_DEFINITONS
…n project, set add_library kind to be module for plugins : seems to define better what it is planned to compile than shared, but it is not true for plugins that will be linked with other plugins
…arget_compile_definitions in most cmake files, wip for definition
@Florent2305
Copy link
Member

It's really cool to improve cmake mechanism and promote "modern cmake" philosophy.
We could discuss later about some points.
For Mathilde and I, we think after the 3.1.1 release it could be the first major PR to look at.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants